-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block Library: Fix incorrect attributes definitions #36264
Conversation
The `templateLock` attribute is mixed between boolean and string. Setting the type as string introduces an regression that will remove `false` on save. The original PR (#36140) tried to fix a validation issue reported in #35902 The schema error is incorrect. The attribute definition is required to have either a `type` or an `enum`.
Good catch. It's a bit surprising to see boolean and string values mixed for this attribute but the proposed fix makes sense 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it with the following unit test:
diff --git a/packages/blocks/src/api/parser/test/get-block-attributes.js b/packages/blocks/src/api/parser/test/get-block-attributes.js
index 727f052a54..7b48db67fd 100644
--- a/packages/blocks/src/api/parser/test/get-block-attributes.js
+++ b/packages/blocks/src/api/parser/test/get-block-attributes.js
@@ -222,6 +222,22 @@ describe( 'attributes parsing', () => {
expect( value ).toBe( 10 );
} );
+ it( 'should return the comment attribute value when using multiple types', () => {
+ const value = getBlockAttribute(
+ 'templateLock',
+ {
+ type: [ 'string', 'boolean' ],
+ enum: [ 'all', 'insert', false ],
+ },
+ '',
+ {
+ templateLock: false,
+ }
+ );
+
+ expect( value ).toBe( false );
+ } );
+
it( 'should reject type-invalid value, with default', () => {
const value = getBlockAttribute(
'number',
We could add it to the test coverage to ensure that the behavior never regresses.
Yes, I'll add that unit test 👍 |
Size Change: +2.72 kB (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, too. Let's merge this in and cherry pick into the 11.9 release.
* Revert "Block Library: Fix incorrect attributes definitions #36140" The `templateLock` attribute is mixed between boolean and string. Setting the type as string introduces an regression that will remove `false` on save. The original PR (#36140) tried to fix a validation issue reported in #35902 The schema error is incorrect. The attribute definition is required to have either a `type` or an `enum`. * Spesify type as string or boolean * Add unit test
* Revert "Block Library: Fix incorrect attributes definitions #36140" The `templateLock` attribute is mixed between boolean and string. Setting the type as string introduces an regression that will remove `false` on save. The original PR (#36140) tried to fix a validation issue reported in #35902 The schema error is incorrect. The attribute definition is required to have either a `type` or an `enum`. * Spesify type as string or boolean * Add unit test
Cherry picked into the Gutenberg 11.9 release in: e8ce5d6 |
Description
The
templateLock
attribute is mixed between boolean and string. Setting the type as string introduces an regression that will removefalse
on save. Addboolean
as a valid type.How has this been tested?
By inserting a block that support templateLock with
false
and confirmed the saved content is correct with the PR applied.Screenshots
Screencast of the error:
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).